Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting default value for date/time picker in Dialog Editor modal #373

Merged
merged 4 commits into from Sep 12, 2019

Conversation

romanblanco
Copy link
Member

@romanblanco romanblanco commented Mar 14, 2019

Adding an option to set default value for date/time picker in Dialog Editor modal:

Screenshot from 2019-03-14 14-19-38

fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1686077

@kedark3
Copy link

kedark3 commented Mar 14, 2019

Just wondering, I had originally reported this BZ, and like to understand, would this allow default date to be dynamically selected as today's date, and every time I open a catalog containing this datetimepicker, I would like it to default to whatever is the date that day, is it possible?

@Loicavenel
Copy link

@kedark3, what is your use case here..

@kedark3
Copy link

kedark3 commented Mar 15, 2019

@Loicavenel in CFME QE team, we are deploying a catalog item, where user can tell us name of provider, template to clone, VM name, service name, and retirement date. In case user does not choose retirement date, it should be set by default to whatever is current date+X hours. I was wondering if we can set that current date+X hours while we create the dialog. Now, in current implementation of our catalog, the default date we see while ordering the catalog is current date and time.

This is an attempt to make sure the VM is removed/retired automatically after certain time if user does not extend it.

Does it make sense? I am not sure if we can design that in datetimepicker or in automate.

@Loicavenel
Copy link

@kedark3 this is more for Automate, you want to introduce a rules that say, if no Retirement selected, automatically take current order date + X

@kedark3
Copy link

kedark3 commented Mar 19, 2019

@Loicavenel thanks for clarifying. Just wanted to make sure if this was not something that we should be doing in this PR/BZ.

@himdel himdel closed this Mar 19, 2019
@himdel himdel reopened this Mar 19, 2019
@himdel
Copy link
Contributor

himdel commented Mar 19, 2019

@romanblanco Are you sure about the BZ in description?

The BZ linked from here says "I can't view bug 1598729", and is marked as closed and a duplicate of https://bugzilla.redhat.com/show_bug.cgi?id=1676724.

The bug mentioned in that BZs subject is a RHEL bug.

The branch name seems most correct, https://bugzilla.redhat.com/show_bug.cgi?id=1686077 is indeed related to the code here :).

Can you fix the description & commit message please, if that's the right bug? :) (Both description and the commit message have the wrong link.)

@himdel
Copy link
Contributor

himdel commented Mar 19, 2019

Verified in UI

both the date and datetime pickers can now have a default value,
the templates are identical except for an extra timepicker for the datetime case.

One nitpick, it's currently impossible to remove the default value using mouse, you have to select the date and press delete. Could we add an x button to clear the default value?

EDIT: and completely impossible for date&time

@himdel
Copy link
Contributor

himdel commented Mar 19, 2019

Uh, one more bug...

have a datetime item,
set a default value,
remove the date in the default value,
try to remove the time (the fields will be red),
switch tabs to Field Information and back to Options

bad

@romanblanco
Copy link
Member Author

@romanblanco Are you sure about the BZ in description?

The BZ linked from here says "I can't view bug 1598729", and is marked as closed and a duplicate of https://bugzilla.redhat.com/show_bug.cgi?id=1676724.

The bug mentioned in that BZs subject is a RHEL bug.

The branch name seems most correct, https://bugzilla.redhat.com/show_bug.cgi?id=1686077 is indeed related to the code here :).

Can you fix the description & commit message please, if that's the right bug? :) (Both description and the commit message have the wrong link.)

The link was incorrect. It is correct now. Thanks!

@romanblanco
Copy link
Member Author

romanblanco commented Apr 3, 2019

Uh, one more bug...

have a datetime item,
set a default value,
remove the date in the default value,
try to remove the time (the fields will be red),
switch tabs to Field Information and back to Options

bad

I'm trying to fix this by using patternfly angular component introduced in patternfly/angular-patternfly#344, used in https://github.com/ManageIQ/manageiq/pull/12165/files#diff-f5af660797161cfea689526173ffec72R92

I'm working on this in WIP branches:
master...romanblanco:bz1686077-v1
ManageIQ/manageiq-ui-classic@master...romanblanco:bz1686077-v1

* @function clearDefaultValue
*/
public clearDefaultValue() {
console.log(this.modalData.default_value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the console.log here?

@romanblanco romanblanco changed the title Allow setting default value for date/time picker in Dialog Editor modal [WIP] Allow setting default value for date/time picker in Dialog Editor modal Apr 5, 2019
@romanblanco romanblanco force-pushed the bz1686077 branch 2 times, most recently from ab3c212 to 5c4b290 Compare September 3, 2019 12:26
@romanblanco
Copy link
Member Author

romanblanco commented Sep 3, 2019

TODO:

  • 'Clear' button works, but error appears:
Error: "[$rootScope:inprog] $digest already in progress
https://errors.angularjs.org/1.6.10/$rootScope/inprog?p0=%24digest"
  • Loading the default_value into the Dialog Editor (Saving works correctly, but loading replaces default_value with today's date)

@romanblanco romanblanco changed the title [WIP] Allow setting default value for date/time picker in Dialog Editor modal Allow setting default value for date/time picker in Dialog Editor modal Sep 9, 2019
@miq-bot miq-bot removed the wip label Sep 9, 2019
<!-- date time control -->
<div ng-switch-when="DialogFieldDateTimeControl">
<div class="col-sm-6 dateTimePadding">
<input ng-model="vm.fieldData.default_value"
pf-datepicker options="vm.fieldData.dateOptions"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dateOptions is intentionally removed? (I'm not seeing it defined anywhere, so sounds about right, just checking :), line 33 does have datepicker-options, line 53 doesn't)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dateOptions is intentionally removed?

yes.

line 33 does have datepicker-options

Thanks! Removing 👍

@himdel
Copy link
Contributor

himdel commented Sep 11, 2019

One last thing... diff between date-control and date-time-control:

--- src/dialog-editor/components/modal-field-template/date-control.html	2019-09-11 13:08:07.824862101 +0000
+++ src/dialog-editor/components/modal-field-template/date-time-control.html	2019-09-11 13:08:07.824862101 +0000
@@ -1,5 +1,12 @@
 <div ng-if="vm.modalTabIsSet('options') && !vm.modalData.dynamic">
   <form class="form-horizontal">
+    <div pf-form-group pf-label="{{'Required'|translate}}">
+      <input bs-switch
+             ng-model="vm.modalData.required"
+             type="checkbox"
+             switch-on-text="{{'Yes'|translate}}"
+             switch-off-text="{{'No'|translate}}">
+    </div>
     <div pf-form-group pf-label="{{'Default value'|translate}}">
       <div class="dateTimePadding">
         <p class="input-group">
@@ -8,16 +15,20 @@
                  class="form-control"
                  ng-model="vm.modalData.default_value"
                  is-open="open"
-                 close-text="{{'Close'|translate}}"/>
+                 min-date="vm.minDate"
+                 close-text="{{'Close'|translate}}"
+                 id="{{ vm.dialogField.name }}"/>
           <span class="input-group-btn">
             <button type="button"
                     class="btn btn-default"
                     ng-click="open = !open">
-              <i class="fa fa-calendar"></i>
-            </button>
+              <i class="fa fa-calendar"></i></button>
           </span>
         </p>
       </div>
+      <div >
+        <uib-timepicker ng-model="vm.modalData.default_value"></uib-timepicker>
+      </div>
     </div>
     <div pf-form-group pf-label="{{'Read only'|translate}}">
       <input bs-switch
@@ -75,6 +86,14 @@
       </dialog-editor-modal-field-template>
     </div>
 
+    <div pf-form-group pf-label="{{'Required'|translate}}">
+      <input bs-switch
+             ng-model="vm.modalData.required"
+             type="checkbox"
+             switch-on-text="{{'Yes'|translate}}"
+             switch-off-text="{{'No'|translate}}">
+    </div>
+
     <dialog-editor-tree-selector ng-if="vm.treeOptions.show"
                                  tree-options="vm.treeOptions">
     </dialog-editor-tree-selector>

Specifically, it seems like date-time-control now has 2 copies of the Required switch, while date-control has none. (Comes from #380)

I assume the only difference between the two should be the extra uib-timepicker?

@romanblanco
Copy link
Member Author

required option is missing in the new template, as it was copied before 1e89fe6 was merged. I'll update the template so it has an option to set required too.

Thanks @himdel

@miq-bot
Copy link
Member

miq-bot commented Sep 11, 2019

Checked commits romanblanco/ui-components@5d43974~...e6f5aae with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 👍

@himdel himdel self-assigned this Sep 12, 2019
@himdel himdel merged commit 8811c76 into ManageIQ:master Sep 12, 2019
@himdel
Copy link
Contributor

himdel commented Sep 12, 2019

LGTM, working :) thanks :)

@himdel himdel added this to the Sprint 120 Ending Sep 16, 2019 milestone Sep 12, 2019
@himdel himdel added the bug label Sep 12, 2019
@romanblanco romanblanco deleted the bz1686077 branch September 13, 2019 10:29
simaishi pushed a commit that referenced this pull request Aug 20, 2020
@simaishi
Copy link

Ivanchuk backport details:

$ git log -1
commit 8660c6fb47a725f3fefb676bdfbd45fa2ab83f80
Author: Martin Hradil <mhradil@redhat.com>
Date:   Thu Sep 12 16:10:20 2019 +0200

    Merge pull request #373 from romanblanco/bz1686077

    Allow setting default value for date/time picker in Dialog Editor modal

    (cherry picked from commit 8811c766d812d37d91cf89f5b455999d81bf20c6)

    https://bugzilla.redhat.com/show_bug.cgi?id=1686077
    https://bugzilla.redhat.com/show_bug.cgi?id=1706848

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants